Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: update theme schema #8

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

feat: update theme schema #8

wants to merge 2 commits into from

Conversation

fedeci
Copy link
Contributor

@fedeci fedeci commented Jan 4, 2022

For backward compatibility we could have a duplicated theme property to point to the default settings. This should be deprecated after one or two versions.
We currently fallback to the default light theme which is embedded in the engine so autocomplete would still work without the themes.

@fedeci fedeci requested a review from mschrage January 4, 2022 20:16
@fedeci fedeci marked this pull request as draft January 5, 2022 20:01
@fedeci
Copy link
Contributor Author

fedeci commented Jan 8, 2022

With this new format we need to add an action to recreate the settings file in macOS repo each time a new config is added or edited. (Similar to what we did for API docs but way simpler)

@brendanfalk
Copy link
Member

Hmmm - want to hear what the other teammates have to say, however, I personally don't think making variants per theme makes too much sense. I think a theme should just be one thing (e.g. light OR dark) and the system theme essentially switches you from one theme to another (rather than a variant within one theme). Then we can eventually let users choose a daytime mode theme and a nighttime mode theme.

I think we are going to start using this themes repo pretty heavily. Not just for autocomplete themes but for mission control themes, terminal themes etc. I can imagine there being hundreds or thousands of themes in the future and almost all of these will just be one variant.

Summary: I agree the schema for themes should be re-imagined, however, I don't believe it should variants makes too much sense..

@fedeci
Copy link
Contributor Author

fedeci commented Jan 11, 2022

I partially agree with you, my intention with these changes was actually just to add light and dark under variants and eventually an high contrast trying to emulate what vscode is doing.
Currently we do not allow theme creators to have both a dark and light version all in one place. While working on system theme I thought it would have been cool to also allow users to create themes that toggle automatically between light and dark versions.
My intention is to provide the setting to configure variants if and only if variants has more than one theme. So the experience for users would not be radically changed.
For sure wanna hear more opinions because this is a super important change that won't probably allow other breaking updates in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants